Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bump oc_version to 28.0.0.1 #39660

Merged
merged 3 commits into from
Aug 7, 2023
Merged

Conversation

hamza221
Copy link
Contributor

@hamza221 hamza221 commented Aug 1, 2023

Summary

Bump oc version to execute this migration

TODO

  • ...

Checklist

Signed-off-by: hamza221 <hamzamahjoubi221@gmail.com>
@hamza221 hamza221 added the 3. to review Waiting for reviews label Aug 1, 2023
@kesselb
Copy link
Contributor

kesselb commented Aug 1, 2023

 1) Test\TextProcessing\TextProcessingTest::testNonexistentTask
Failed asserting that exception of type "TypeError" matches expected exception "OCP\Common\Exception\NotFoundException". Message was: "OC\TextProcessing\Manager::getTask(): Argument #1 ($id) must be of type int, float given, called in /__w/server/server/tests/lib/TextProcessing/TextProcessingTest.php on line 262" at
/__w/server/server/lib/private/TextProcessing/Manager.php:170
/__w/server/server/tests/lib/TextProcessing/TextProcessingTest.php:262

@marcelklehr could you take a look?

The failure is from our 32bit test suite.
98765432456 is too big for an integer on 32bit PHP.

Quick: Change the value for the test case to or lower 2147483647

Nicer: Update the signature for Manager.getTask and TaskMapper.find to accept int|float.

@marcelklehr
Copy link
Member

mmh, that 32bit CI workflow wasn't there when I did the original PR.... 🤔

@kesselb
Copy link
Contributor

kesselb commented Aug 4, 2023

Sorry @marcelklehr, I sent you on the wrong track.

We use int|float sometimes in server for the files handling on 32bit, but not for http requests / database ids.
Please adjust the test to use a lower number and use int as type for the ids.

Signed-off-by: Marcel Klehr <mklehr@gmx.net>
@marcelklehr marcelklehr force-pushed the fix/llm-migration-not-executed branch from 98b97e5 to f62711d Compare August 7, 2023 10:03
@kesselb kesselb added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Aug 7, 2023
@julien-nc julien-nc merged commit b4a3f80 into master Aug 7, 2023
24 of 25 checks passed
@julien-nc julien-nc deleted the fix/llm-migration-not-executed branch August 7, 2023 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: LLM text processing API fails because migration is not executed automatically
5 participants